Skip to content

Support SizeDict import in get_size_dict#44903

Merged
hmellor merged 3 commits intohuggingface:mainfrom
yonigozlan:more-im-proc-refactor-fixes
Mar 23, 2026
Merged

Support SizeDict import in get_size_dict#44903
hmellor merged 3 commits intohuggingface:mainfrom
yonigozlan:more-im-proc-refactor-fixes

Conversation

@yonigozlan
Copy link
Copy Markdown
Member

What does this PR do?

Some remote code models are using get_size_dict directly, and now that size is converted to SizeDict in init, we need to support it as input in get_size_dict

@yonigozlan yonigozlan requested a review from hmellor March 21, 2026 01:25
@HuggingFaceDocBuilderDev
Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Comment on lines 603 to 606
size (`int | Iterable[int] | dict[str, int] | SizeDict`, *optional*):
The `size` parameter to be cast into a size dictionary.
max_size (`int | None`, *optional*):
The `max_size` parameter to be cast into a size dictionary.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that size and max_size are mutually exclusive? If yes you should add an error if they are both non-None

Comment on lines +555 to +556
if isinstance(size, dict):
return size
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block will never be entered because of

    if not isinstance(size, dict):
        size_dict = convert_to_size_dict(size, max_size, default_to_square, height_width_order)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes maybe I was overly cautious there, since users used get_size_dict directly which I didn't expect, I was thinking that maybe some also import and use convert_to_size_dict directly. But dict and SizeDict inputs weren't supported originally in convert_to_size_dict so that's probably not necessary.
Will revert this and move the SizeDict check to get_size_dict

Comment on lines +557 to +558
if isinstance(size, SizeDict):
return dict(size)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be a condition in get_size_dict? It might be confusing if we end up entering this function with non-default values of max_size, default_to_square, height_width_order and they have no effect.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes indeed, I'll revert this as mentioned above

Copy link
Copy Markdown
Member

@hmellor hmellor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If size is already a

@hmellor hmellor added this pull request to the merge queue Mar 23, 2026
Merged via the queue into huggingface:main with commit 687a70d Mar 23, 2026
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants